Support deploy a service application via aenv cli #53
Conversation
JacksonMei
commented
Jan 19, 2026
JacksonMei
commented
- fix aenv instance bug
- show better list table
- Support deploy a service application with specified port and storage via aenv cli
Summary of ChangesHello @JacksonMei, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a major new capability to the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces significant new functionality to support deploying service applications, a major enhancement to the aenv CLI. The changes are extensive, spanning the Python CLI, Go backend services, and Kubernetes configurations. Overall, the implementation is solid. I particularly appreciate the refactoring in the CLI, which has greatly improved code organization, error handling, and user experience with better table formatting and more informative error messages. The backend changes to support services, including the new controllers and models, are well-structured. I've identified a few areas for improvement, mainly concerning consistency in error handling and retry logic in the client, code duplication in the new CLI commands, and a hardcoded value in the Kubernetes configuration that could affect portability. Addressing these points will further enhance the quality and robustness of this new feature.
| else: | ||
| return [] |
There was a problem hiding this comment.
This method silently returns an empty list [] when the API call is unsuccessful (i.e., api_response.success is false). This can be misleading for the caller, as it hides potential issues like authentication failures or server-side errors, making them indistinguishable from the case where no services actually exist. This behavior is inconsistent with other methods like get_env_service which raise an EnvironmentError. To ensure robust error handling, this should be changed to raise an EnvironmentError with the error message from the API response.
| else: | |
| return [] | |
| else: | |
| error_msg = api_response.get_error_message() | |
| raise EnvironmentError(f"Failed to list services: {error_msg}") |
| ) from e | ||
|
|
||
| except httpx.RequestError as e: | ||
| import random |
There was a problem hiding this comment.
The import random statement is located inside an except block. According to PEP 8, imports should generally be at the top of the file. This improves readability and makes dependencies clear. Please move this import to the top of the file.
|
|
||
| except httpx.RequestError as e: | ||
| if attempt < self.max_retries: | ||
| wait_time = 2**attempt |
There was a problem hiding this comment.
The retry backoff logic is inconsistent across different methods in this class. The create_env_service method uses exponential backoff with jitter (2**attempt + random.uniform(0, 1)), which is a best practice to prevent thundering herd issues. However, this method and others (list_env_services, delete_env_service, update_env_service) use simple exponential backoff without jitter. For consistency and robustness, I recommend applying the same jittered backoff strategy to all retry mechanisms in this client.
| wait_time = 2**attempt | |
| wait_time = 2**attempt + random.uniform(0, 1) |
| def _load_env_config() -> Optional[Dict[str, Any]]: | ||
| """Load build configuration from config.json in current directory. | ||
|
|
||
| Returns: | ||
| Dictionary containing build configuration, or None if not found. | ||
| """ | ||
| config_path = Path(".").resolve() / "config.json" | ||
| if not config_path.exists(): | ||
| return None | ||
|
|
||
| try: | ||
| with open(config_path, "r") as f: | ||
| config = json.load(f) | ||
| return config | ||
| except Exception: | ||
| return None |
There was a problem hiding this comment.
The helper function _load_env_config is duplicated here and in aenv/src/cli/cmds/instance.py. To improve code reuse and maintainability, this function should be moved to a shared utility module, such as aenv/src/cli/utils/api_helpers.py, and imported where needed. The same applies to the _get_system_url function.
| except Exception as e: | ||
| console.print(f"[red]❌ Creation failed:[/red] {str(e)}") | ||
| if cfg.verbose: | ||
| import traceback | ||
| console.print(traceback.format_exc()) | ||
| raise click.Abort() |
There was a problem hiding this comment.
The exception handling in the create, delete, and update commands is very generic, catching Exception and printing a general failure message. This contrasts with the more detailed error handling in the list and get commands in this file, and in the refactored instance command, which provide specific, user-friendly feedback for common issues like authentication or connection errors. I recommend enhancing the error handling here to catch specific exceptions and provide more informative messages to the user, which will significantly improve the CLI's usability.
deploy/controller/values.yaml
Outdated
| spec: | ||
| accessModes: | ||
| - ReadWriteOnce | ||
| storageClassName: alilocal-ssd |
There was a problem hiding this comment.
The storageClassName is hardcoded to alilocal-ssd. This reduces the portability of the Helm chart, as it will fail in Kubernetes clusters that do not have this specific StorageClass. It's better to either omit this field to use the cluster's default StorageClass, or make it a configurable value in values.yaml.
Remove unnecessary nil checks before ranging over maps as ranging over nil maps is safe in Go and will simply skip iteration. Co-Authored-By: Claude <noreply@anthropic.com>
